Skip to content

feat(desktop): consume task submission readiness - #2523

Merged
Astro-Han merged 4 commits into
apache:mainfrom
liugddx:feat/desktop-task-readiness-consumer
Aug 9, 2026
Merged

Astro-Han merged 4 commits into
apache:mainfrom
liugddx:feat/desktop-task-readiness-consumer

Conversation

@liugddx

@liugddx liugddx commented Aug 8, 2026

Copy link
Copy Markdown
Member

English

What changed

  • Query the authoritative task-submission readiness snapshot for the active session or new-task target.
  • Disable submission only for confirmed repair_required or unavailable states; unknown and loading states remain non-blocking.
  • Show actionable runtime/workspace recovery notices above the composer, with retry or new-task actions.
  • Keep model recovery owned by the existing model/connection surfaces to avoid duplicate guidance and preserve model switching.
  • Clear stale readiness immediately when the model or workspace target changes.

Validation

  • npx biome check (changed files)
  • Desktop main, preload, and renderer builds
  • Desktop renderer and preload TypeScript checks
  • 17 targeted Core/Main/Renderer readiness tests

Known baseline issue

  • The aggregate workspace dependency build currently stops in @maka/ui at src/tool-activity.tsx because ChatToolCallItem does not expose activateLabel / onActivate. Desktop builds pass after rebuilding the other workspace dependencies; this PR does not touch that code.

中文

改动内容

  • 针对当前会话或新任务目标,读取权威的任务提交 readiness 快照。
  • 仅在状态明确为 repair_requiredunavailable 时禁止提交;unknown 和加载中不会阻塞用户。
  • 在输入区上方显示可操作的运行时/工作区恢复提示,并提供重试或新建任务操作。
  • 模型问题继续由现有模型/连接恢复界面负责,避免重复提示,同时保留切换模型能力。
  • 切换模型或工作区时立即清除旧 readiness,防止旧错误误锁新目标。

验证

  • 变更文件 Biome 检查
  • Desktop main、preload、renderer 构建
  • Desktop renderer、preload TypeScript 检查
  • 17 个 Core/Main/Renderer readiness 针对性测试

已知基线问题

  • 当前聚合 workspace 依赖构建会在 @maka/ui/src/tool-activity.tsx 停止,因为 ChatToolCallItem 尚未暴露 activateLabel / onActivate。重建其余依赖后 Desktop 构建均通过;本 PR 未修改该处。

Refs #2497
Related to #2469

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks—this is a good direction. Two non-blocking P2 suggestions:

  • For an unlocked stale session, this can disable Send even though the existing send authority would rebind it to a healthy connection. Please reuse the session-send projection.
  • Readiness is cached by the hook and not refreshed when Send is clicked, so runtime/workspace changes can bypass or stale the gate. Consider a final send-time check while keeping unknown fail-open.

P3: route the workspace action from repairTarget; “New task” can be a no-op when already on that surface.

@liugddx

liugddx commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Addressed all three suggestions in 4405412ce:

  • Active-session readiness now consumes the existing sessionSendOutcome; an unlocked stale Session checks the projected rebind target, and model readiness does not override that authority while the projection is loading/ready/rebinding.
  • Ordinary, Graph, and Swarm submissions perform a fresh readiness read immediately before the real send. Confirmed blockers stop; unknown remains fail-open. Concurrent reads are sequenced so stale responses cannot overwrite the latest result.
  • Workspace recovery now follows the snapshot repairTarget. From an active Session it opens the new-task surface; when already there it opens workspace addition/selection instead of issuing a no-op “New task”.

Added the stale-session rebind behavior case; 18 targeted readiness tests, Desktop main build, and Renderer typecheck pass.

已在 4405412ce 处理三点:复用 session-send projection、每次真实发送前重新检查且 unknown 放行、并按 repairTarget 路由工作区恢复操作。

@liugddx
liugddx requested a review from Astro-Han August 9, 2026 03:57
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for addressing the previous round — I should have been more explicit about ownership in my send-time suggestion. The remaining issue is not the refresh itself, but keeping authorization and execution bound to the same submission target.

  • P2: The readiness await currently runs before send() captures initialSessionId. If the user switches sessions while that read is pending, readiness is checked for session A, but the stable send facade can then submit the text and attachments to session B.

The smallest patch would be to capture the composer owner before the await and cancel if it changes. The cleaner fix, and my preference, is to move the readiness guard into createAppShellChatActions.send(), immediately after it captures initialSessionId / sendOwner, and remove the three call-site checks. That keeps capture → readiness → send under one owner and preserves the existing navigation semantics. A deferred-read regression that switches sessions before resolving would lock this down.

@liugddx

liugddx commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Fixed in de8f00830 using the preferred ownership boundary:

  • createAppShellChatActions.send() now captures initialSessionId / composer owner first, then awaits the readiness guard, then verifies that the same owner is still active before any Session creation, turn arm, attachment ingest, or sessions.send.
  • Removed the three ordinary / Graph / Swarm call-site checks; every path now goes through the single stable send guard.
  • Added a deferred-read regression: readiness starts for Session A, navigation switches to Session B before resolution, and the send returns false with zero sessions.send calls.

22 related tests pass, along with Desktop main build and Renderer typecheck.

已在 de8f00830 将 readiness 检查绑定到稳定 send owner,并增加延迟读取期间切换 Session 的回归测试。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for moving the readiness check into the stable send() ownership boundary. The previous P2 is resolved: the owner is captured before the await, revalidated before any Session creation, attachment ingest, or send, and ordinary/Graph/Swarm submissions now share the same guard. The deferred Session-switch regression covers the original failure well.

One optional P3: requestSequence prevents a superseded checkNow() result from overwriting UI state, but the old call still returns its snapshot to the send-time decision. Two overlapping reads that settle out of order can therefore briefly disagree about whether to send. It may be worth returning/carrying the generation and cancelling when the send-time result has been superseded.

This is timing-dependent and remains fail-safe at the Host authority, so it does not block this PR. All checks are green and I found no remaining P1/P2 issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants